Skip to content

✨ feat(forwardproxy): support HTTPS CONNECT as raw TCP passthrough#430

Merged
huang195 merged 2 commits into
rossoctl:mainfrom
huang195:feat/forwardproxy-https-connect
May 22, 2026
Merged

✨ feat(forwardproxy): support HTTPS CONNECT as raw TCP passthrough#430
huang195 merged 2 commits into
rossoctl:mainfrom
huang195:feat/forwardproxy-https-connect

Conversation

@huang195

Copy link
Copy Markdown
Member

Summary

The forward proxy previously rejected every HTTPS CONNECT with 405 Method Not Allowed. After kagenti-operator commit 21a2258 (May 14, 2026) flipped the cluster default mode from envoy-sidecar to proxy-sidecar, this broke external HTTPS for every agent in CI: the operator injects HTTPS_PROXY=http://127.0.0.1:<port> on every agent container, Python's openai/httpx client honors it and sends CONNECT litellm-prod...:443 to the forward proxy, the 405 propagates back, and the agent reports openai.APIConnectionError: Connection error. This was the actual root cause of issue #428 (the original "proxy-init iptables" diagnosis was misframed — the failing pods are in proxy-sidecar mode and have no proxy-init container at all).

This PR implements CONNECT as a raw TCP tunnel — matching envoy-sidecar's existing TLS-passthrough behavior. The bytes flowing through the tunnel are the agent's end-to-end TLS, opaque to the proxy by definition, so the protocol parsers (mcp-parser, inference-parser) and token-exchange are no-ops on this path. Pipeline gates that policy on host/identity (ibac, etc.) still run on the CONNECT request itself and can deny based on destination host before the tunnel opens.

Key design points

  • Pipeline first: outbound pipeline runs on the CONNECT request before the upstream is dialed. Reject paths use the existing httpx.WriteRejection + recordOutboundReject machinery, so abctl / /v1/sessions surfaces denied CONNECTs the same way as denied HTTP POSTs.
  • Hijack before dial: hijack the client conn before opening the upstream so a hijack failure produces a clean 500 instead of a half-opened upstream socket.
  • mTLS deliberately not applied: the bytes flowing through this tunnel ARE the agent's own end-to-end TLS. Terminating that with sidecar-to-sidecar mTLS would break the agent's trust path. CONNECT targets are opaque externals (LiteMaaS, Bedrock, GitHub API, etc.) where the agent's TLS is the right answer. The mtlsDialer continues to apply only to the plain-HTTP forwarding path (where it actually makes sense — sidecar-to-sidecar plaintext upgrade).
  • Bidirectional copy with explicit Close on both sides: net.Conn.Close is idempotent so the duplicate close on the loser side is harmless.
  • Dial timeout bounded at 30s; once tunnel is up no timeout (agent's TLS handshake + subsequent traffic flow at their own pace).

Tests

Test What it asserts
TestForwardProxy_CONNECT_TunnelsBytes Real TCP echo upstream. After CONNECT 200, bytes round-trip both directions through the tunnel (helloecho:hello).
TestForwardProxy_CONNECT_PipelineDeny Pipeline rejects before upstream dial; client sees a non-200 status line. Plugin policy governs CONNECT, not just HTTP.
TestForwardProxy_CONNECT_BadGatewayOnDialFailure Upstream refuses; client sees 502, not a half-opened tunnel.

The pre-existing TestForwardProxy_CONNECT_Rejected (which asserted 405) is replaced by these three.

Verification

  • go test -race -count=1 ./authlib/... — all packages green
  • go build ./cmd/{authbridge-proxy,authbridge-envoy,authbridge-lite,abctl} — clean
  • go vet, gofmt — clean on touched files

Out of scope

  • kagenti-operator change to drop HTTPS_PROXY injection — also a valid fix path (option A in the investigation), but with this PR's CONNECT support, the existing HTTPS_PROXY injection becomes correct rather than wrong, so no operator change is needed.
  • Token-exchange for HTTPS targets — fundamentally not possible (the bytes are encrypted). Agents that need outbound token exchange must continue to use plain HTTP outbound, which is unchanged by this PR.

Closes

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

The forward proxy previously rejected every CONNECT request with 405,
which meant agents in proxy-sidecar mode couldn't reach external HTTPS
endpoints — the kagenti-operator injects HTTPS_PROXY env var on every
agent container, Python's openai/httpx honors it and sends CONNECT to
the forward proxy, and the 405 surfaced to the agent as
`openai.APIConnectionError: Connection error.`

That was the root cause of every HyperShift CI weather-agent failure
since kagenti-operator commit 21a2258 (May 14) flipped the cluster
default mode from envoy-sidecar to proxy-sidecar. Pre-flip, Envoy's
TLS-passthrough filter chain handled external HTTPS transparently;
post-flip, the forward proxy was the only outbound path and it refused.

Implement CONNECT as a raw TCP tunnel — match envoy-sidecar's
TLS-passthrough behavior. The bytes flowing through a CONNECT tunnel
are the agent's end-to-end TLS, opaque to the proxy by definition, so
the protocol parsers (mcp-parser, inference-parser) and token-exchange
are no-ops on this path. Pipeline gates that policy on host/identity
(ibac, etc.) still run on the CONNECT request itself and can deny
based on destination host before the tunnel opens.

Implementation notes:

  - Run the outbound pipeline first. Reject paths use the existing
    httpx.WriteRejection + recordOutboundReject machinery so abctl /
    /v1/sessions surfaces denied CONNECTs the same way as denied HTTP
    POSTs.
  - Hijack BEFORE dialing upstream so a hijack failure produces a
    clean 500 instead of a half-opened upstream socket. Dial timeout
    bounded at 30s; once tunnel is up no timeout (the agent's TLS
    handshake and subsequent traffic flow at their own pace).
  - mTLS is intentionally NOT applied to the upstream dial. The bytes
    flowing through this tunnel ARE the agent's own end-to-end TLS;
    terminating that with sidecar-to-sidecar mTLS would break the
    agent's trust path. CONNECT targets are opaque externals (LiteMaaS,
    Bedrock, GitHub API, etc.) where the agent's TLS is the right
    answer.
  - Bidirectional copy with explicit Close on both sides — net.Conn
    Close is idempotent so the duplicate close on the loser-side is
    harmless.

Tests:

  - TestForwardProxy_CONNECT_TunnelsBytes: real TCP echo upstream,
    verifies CONNECT 200, then bytes round-trip both directions
    through the tunnel.
  - TestForwardProxy_CONNECT_PipelineDeny: pipeline rejects before
    dial; client sees a non-200 status line. Plugin policy still
    governs CONNECT.
  - TestForwardProxy_CONNECT_BadGatewayOnDialFailure: upstream
    refuses; client sees 502 (not a half-opened tunnel).

The pre-existing TestForwardProxy_CONNECT_Rejected (which asserted 405)
is replaced by these three.

Verified:
  - go test -race ./authlib/... — all packages green.
  - go build ./cmd/{authbridge-proxy,authbridge-envoy,authbridge-lite,abctl} — clean.
  - go vet, gofmt — clean.

Closes rossoctl#428.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Four review comments, all applied:

1. Observability gap: allowed CONNECTs were silent in /v1/sessions
   while denied ones were recorded via recordOutboundReject. Added a
   SessionRequest event after the pipeline allows + before the tunnel
   opens, mirroring the HTTP path's post-Allow recording. Same gating
   as the HTTP path (record only when Invocations or plugin-public
   Plugins entries are non-nil) so we don't spam empty events for
   pipelines that don't actually run gate plugins on outbound.

2. Misleading comment: "Hijack BEFORE dialing upstream" implied the
   hijacker.Hijack() call happens before net.DialTimeout, but it
   doesn't (and shouldn't — http.Error needs an un-hijacked
   ResponseWriter to deliver the dial-failure 502). The PR review
   author is right that what we actually do is verify hijack
   capability via type-assertion before dialing, then hijack only
   after dial succeeds. Rewrote the comment to say so and moved the
   actual Hijack() call to use w.(http.Hijacker) directly so the
   type-assertion-as-capability-check is visually distinct from the
   commit-to-tunnel hijack.

3. Missing TCP keepalive: streaming LLM completions can hold the
   tunnel open for minutes, and a vanished peer (NAT entry expiry,
   peer reboot, network partition) parks the io.Copy goroutines
   until the OS finally times the socket out. Added enableKeepalive
   helper that sets SO_KEEPALIVE + 30s probe interval on both the
   upstream conn and the hijacked client conn. No-op when the conn
   doesn't unwrap to *net.TCPConn (defensive).

4. Single-roundtrip test: a real TLS handshake is multi-RTT with
   interleaved reads and writes; one write/read in
   TestForwardProxy_CONNECT_TunnelsBytes catches basic plumbing but
   could miss a half-duplex regression in the bidirectional copy.
   Reworked the upstream echo loop to keep accepting and added a
   three-payload round-trip in the test ("hello", "world", "third").

Verified:
  go test -race -count=1 ./authlib/... — green
  go vet, gofmt — clean

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid implementation of CONNECT tunneling. The design is well-reasoned: pipeline runs before dial, hijack capability is checked early, keepalives handle idle detection, and the bidirectional copy pattern is standard Go TCP proxying. Tests cover the happy path (multi-round-trip), policy rejection, and upstream failure — good edge case coverage.

Areas reviewed: Go (implementation + tests), Security, Commit Conventions
Commits: 2 commits, all signed-off ✓
CI status: all checks passing ✓

_, _ = io.Copy(upstream, clientConn)
_ = upstream.Close()
_ = clientConn.Close()
}()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: io.Copy errors are silently discarded in both directions. Consider logging at slog.Debug level on non-EOF errors — it won't affect correctness but helps diagnose weird disconnects during incident triage (e.g. a network partition that leaves one side writeable but the other already RST'd).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug: proxy-init OUTBOUND_PORTS_EXCLUDE missing port 443 — blocks external HTTPS from agent pods

3 participants